Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

feature: qchat #182

Merged
merged 112 commits into from
Aug 30, 2024
Merged

feature: qchat #182

merged 112 commits into from
Aug 30, 2024

Conversation

gounux
Copy link
Contributor

@gounux gounux commented Jul 18, 2024

No description provided.

@github-actions github-actions bot added enhancement New feature or request UI User interface: forms, widgets... labels Jul 18, 2024
@gounux gounux marked this pull request as ready for review July 21, 2024 13:08
qtribu/metadata.txt Outdated Show resolved Hide resolved
@Guts
Copy link
Member

Guts commented Jul 23, 2024

Super travail, je le redis, je trouve ça génial 👏 !

Quelques remarques / idées en vrac :

  • déplacer le nickname dans le widget d'authoring je trouve cela plus cohérent et on peut aussi imaginer utiliser le nickname mastodon, Github, LinkedIn ou Twitter (dans l'ordre) comme fallback si le champ nickname n'est pas rempli
  • remplacer l'icône warning par un truc moins anxiogène. Peut-être QgsApplication.getThemeIcon("processingResult.svg) ou QgsApplication.getThemeIcon("mIconCodeEditor.svg) qui m'évoquent un document à lire
  • dans le dock widget, ajouter un bouton pour ouvrir les paramètres du plugin
  • il est possible d'utiliser un espace ou autre comme nickname : ajouter une longueur minimale voire une regex de validation (dans https://github.com/geotribu/qtribu/blob/main/qtribu/gui/gui_commons.py)
  • il est possible de changer de nickname entre chaque message. Je sais pas si c'est pas un peu trop libertaire. Ne pas permettre d'éditer le nickname directement dans le dockwidget, seulement dans les paramètres
  • le bouton et surtout le dialog status sont overkill pour le niveau d'info. Pourquoi ne pas ajouter le nombre d'utilisateurs directement dans la combobox des rooms avec une MAJ triggered à chaque fois qu'elle est déroulée/activée ? Et puis ça fait 2 fois status mais qui servent pas à la même chose
  • la date, c'est celle du serveur ? de l'ordinateur local ? comme c'est géré quand des utilisateurs de différents fuseaux échangent des messages ?
  • colorer le fond des messages admin différemment
  • idem pour ses propres messages
  • appliquer la couleur de sélection de qgis quand un message contient le nickname enregistré (oui, oui on aime la regex)
  • rendre l'URL de l'instance cliquable
  • ajouter une room "QGIS Dév"
  • ajouter une option pour être notifié s'il y a une mention de son nickname (@geojulien). Ne paz s'emmerder avec la casse
  • changer l'instance dans les paramètres ne la change pas dans le dockwidget

@gounux
Copy link
Contributor Author

gounux commented Jul 24, 2024

Merci pour ta review @Guts !

il est possible d'utiliser un espace ou autre comme nickname : ajouter une longueur minimale voire une regex de validation (dans https://github.com/geotribu/qtribu/blob/main/qtribu/gui/gui_commons.py)

Partons sur minimum 3, sans espace et caractères alphanumériques seulement, ça devrait suffire je pense

le bouton et surtout le dialog status sont overkill pour le niveau d'info. Pourquoi ne pas ajouter le nombre d'utilisateurs directement dans la combobox des rooms avec une MAJ triggered à chaque fois qu'elle est déroulée/activée ? Et puis ça fait 2 fois status mais qui servent pas à la même chose

C'est une bonne remarque, à voir pour une première version. Si on part là-dedans autant faire transiter des messages de fonctionnement interne dans la websocket (j'ai créé une issue à ce sujet). Ça pourrait être pas mal d'utiliser l'author "internal" pour envoyer des messages lors de connexions / déconnections avec le nombre de personnes dans la room, côté back. Et modifier le titre de la groupbox du chat en conséquence lors de la réception d'un tel message :

image

la date, c'est celle du serveur ? de l'ordinateur local ? comme c'est géré quand des utilisateurs de différents fuseaux échangent des messages ?

Je me suis également posé cette question, au début la date du message était générée par le serveur mais en fin de compte je crois que c'est plus simple de l'afficher côté client lorsque le message arrive, donc date locale. Mais les américains seront peut-être plus à l'aise avec un format type 4:20:05 PM plutôt que 16:20:05 comme c'est le cas à l'heure actuelle

rendre l'URL de l'instance cliquable

Galère galère

changer l'instance dans les paramètres ne la change pas dans le dockwidget

On touche aux subtilités des signaux Qt. J'ai fait des modifs de sorte à toujours lire les settings depuis la PlgSettingsStructure à jour (initialement les settings étaient chargés lors du open du widget). Concrètement pour changer l'affichage dans le label de l'instance, il est nécessaire de fermer puis rouvrir le widget, ou bien en cliquant sur le bouton Settings du widget ça le fait aussi

@gounux
Copy link
Contributor Author

gounux commented Jul 24, 2024

@Guts niveau UI ça donne ça :

image

À noter que certains UI themes de QGIS désactivent la possibilité de coloration des messages, enfin j'ai l'impression :

image

Allez ces couleurs ça peut partir en paramétrable dans les settings

@Guts
Copy link
Member

Guts commented Jul 24, 2024

En testant sur un qgis fraichement installé sur Ubuntu 22.04, ça plante sur l'import de QtWebSockets :

image

Il faut gérer l' ImportError et désactiver la fonctionnalité liée avec un message. Un peu comme sur QSoccer, Thyrsis, QDuckDB etc.

@Guts
Copy link
Member

Guts commented Jul 24, 2024

Je me suis également posé cette question, au début la date du message était générée par le serveur mais en fin de compte je crois que c'est plus simple de l'afficher côté client lorsque le message arrive, donc date locale. Mais les américains seront peut-être plus à l'aise avec un format type 4:20:05 PM plutôt que 16:20:05 comme c'est le cas à l'heure actuelle

Normalement pour ce genre de service tu récupères le timestamp en UTC puis ça se formate bien avec Qt en tenant compte des paramètres de l10n de l'application.

On touche aux subtilités des signaux Qt. J'ai fait des modifs de sorte à toujours lire les settings depuis la PlgSettingsStructure à jour (initialement les settings étaient chargés lors du open du widget). Concrètement pour changer l'affichage dans le label de l'instance, il est nécessaire de fermer puis rouvrir le widget, ou bien en cliquant sur le bouton Settings du widget ça le fait aussi

Je verrais bien ce workflow :

  • quand les settings sont sauvegardés, on check si l'instance ou le nickname ont changé
  • s'ils l'un deux a changé :
    • on déconnecte la session de tchat et
    • on change le label avec l'URL de l'instance

C'est une bonne remarque, à voir pour une première version. Si on part là-dedans autant faire transiter des messages de fonctionnement interne dans la websocket (j'ai créé geotribu/gischat#9 à ce sujet). Ça pourrait être pas mal d'utiliser l'author "internal" pour envoyer des messages lors de connexions / déconnections avec le nombre de personnes dans la room, côté back. Et modifier le titre de la groupbox du chat en conséquence lors de la réception d'un tel message :

Excellente idée !

@Guts niveau UI ça donne ça :

image

Je mettrais bien le bouton connect/disconnect à droite de la liste déroulante des rooms.

À noter que certains UI themes de QGIS désactivent la possibilité de coloration des messages, enfin j'ai l'impression :

image

Alors ça on s'en tamponne le coquillard comme on dit dans les réunions de frêt routier à propos des temps de repos.

Allez ces couleurs ça peut partir en paramétrable dans les settings

Le gars est tellement bouillant !

qtribu/constants.py Outdated Show resolved Hide resolved
@github-actions github-actions bot added the documentation Improvements or additions to documentation label Jul 24, 2024
@gounux
Copy link
Contributor Author

gounux commented Jul 31, 2024

@Guts on merge ?

@Guts
Copy link
Member

Guts commented Jul 31, 2024

@Guts on merge ?

Perso, je préfère attendre car comme je te disais sur Signal, Je pense qu'il faut d'abord faire une version stabilisée à partir de main. Il y a notamment 2/3 ch'touilles qui traînent :

  • la gestion de QT Web engine (ping @lbartoletti),
  • l'image de l'intégration au newsfeed
  • et autres poussières

Sinon, on ne pourra plus publier de version sans intégrer le tchat dedans.

Voilà c'est mon avis. Mais j'approuve quand même la PR pour te laisser libre cours.

Copy link

sonarcloud bot commented Aug 29, 2024

@Guts Guts merged commit d513bb9 into main Aug 30, 2024
11 checks passed
@Guts Guts deleted the feature/qchat branch August 30, 2024 07:04
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
documentation Improvements or additions to documentation enhancement New feature or request UI User interface: forms, widgets...
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants